Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Support Field Revalidation #9

Merged
merged 8 commits into from
May 5, 2024
Merged

Feat: Support Field Revalidation #9

merged 8 commits into from
May 5, 2024

Conversation

ITenthusiasm
Copy link
Member

@ITenthusiasm ITenthusiasm commented May 5, 2024

We're finally adding support for field revalidation, as it seems like this is an important feature to have in form validation tools. Closes #8.

Highlights:

  • Arrays are no longer supported for the FormValidityObserver's type constructor argument.
  • You can now pass null to the type argument of the FormValidityObserver to use it in "Manual Mode".
  • Field revalidation is now supported via the revalidateOn option.
  • The useFormValidityObserver React hook is being removed because it isn't genuinely useful.

The "Manual Mode" feature and the revalidation feature can be combined to allow developers to validate form fields oninput, but only after a form submission has occurred.

const observer = new FormValidityObserver(null, { revalidateOn: "input" });
const form = document.querySelector("form");
observer.observe(form);

// Prevent the browser from creating error bubbles `onsubmit` (optional)
form.setAttribute("novalidate", "");
form.addEventListener("submit", handleSubmit);

function handleSubmit(event) {
  event.preventDefault();
  // Enables revalidation for all form fields. (Can be disabled by using the `enableRevalidation: false` option.)
  const success = observer.validateFields({ focus: true });

  if (success) {
    // Submit data to server
  }
}

We're excited about these changes and eager to see how things play out from here!

Additional Details

Below are more details on some of the significant design decisions that we made. You can also find these details in our DESIGN_DECISIONS markdown file.

Only Allow One (1) Event Type to Be Passed to the FormValidityObserver Constructor

Although this is a breaking change, it is one that will more easily allow us to support the revalidateOn feature that we mentioned on GitHub while also maintaining the parent-child relationship between the FormObserver and the FormValidityObserver. (We could technically choose to break that relationship, but the reusable observe and unobserve methods seem to be proving helpful right now.)

It technically isn't that difficult to continue supporting an array of strings for the types argument of the FormValidityObserver constructor. But the (very small) additional effort for this support did not seem worth it.

Practically speaking, most (if not all) developers are likely going to choose only one event type anyway. And they'll have to choose one of the events that actually relate to user interaction (so that the form is only validated as users interact with it). The most common of these events are input, focusout (the bubbling version of the blur event), and change. But each of these events renders the other ones irrelevant.

For example, the last input event that's triggered for a form field will always be triggered before change or focusout. Consequently, it doesn't make sense to add change or focusout to the types argument of the FormValidityObserver constructor when input is supplied. The developer would only need to specify the input event itself.

Similarly, the change event is only fired a subset of the times that the focusout event is fired. (There are technically exceptions to this rule, such as <select> and <input type="checkbox">. But practically speaking, this rule holds for all form controls -- including those.) So instead of specifying both events, a developer would only need to specify the focusout event. Finally, if a developer wants to respond to change events exclusively, then they will only specify that event. Neither focusout nor input could be added in that scenario because it would cause the form controls to be validated too frequently.

In the end, whichever event type is chosen, it is only practical for the developer to specify 1 event. Technically speaking, developers could try to specify custom events. But those events would likely be emitted in response to input/focusout/change events anyway. So everything ultimately comes down to those 3 events.

In conclusion, it will be simpler for us to only allow 1 event to be specified for the types argument moving forward. Since this is also what will happen naturally (and since enforcing this might protect some developers from unexpected footguns), this seems like a good change.

Deprecate the useFormValidityObserver React Hook

Originally, we thought that it would be a good idea to memoize the FormValidityObserver for the React developers who would use our tool. That was the idea behind our useFormValidityObserver hook: It would enable developers to use the FormValidityObserver without having to think about memoization. It's a great idea! And when possible, I think that maintainers should take this route! But unfortunately, this approach is not always an option...

Consider a scenario where a developer uses the useFormValidityObserver hook with a complex configuration:

import { useFormValidityObserver } from "@form-observer/react";

function MyForm() {
  const { configure, ...rest } = useFormValidityObserver("focusout", {
    revalidateOn: "input",
    renderer(errorContainer, errorMessage) {
      // Implementation ...
    },
    defaultErrors: {
      // Default error message configuration ...
    },
  });
}

The result returned by useFormValidityObserver is memoized. However, the useMemo hook (which the function calls internally) uses the function's type argument and object options as its dependencies. If the MyForm component re-renders for any reason, the "focusout" and "input" strings will remain constant. But the renderer function and the defaultErrors object will be re-defined during re-renders, causing the useMemo hook to create a new FormValidityObserver. So those options need to be memoized.

import { useMemo, useCallback } from "react";
import { useFormValidityObserver } from "@form-observer/react";

function MyForm() {
  const { configure, ...rest } = useFormValidityObserver("focusout", {
    revalidateOn: "input",
    renderer: useCallback((errorContainer, errorMessage) => {
      // Implementation
    }, []),
    defaultErrors: useMemo(
      () => ({
        // ... Default error message configuration
      }),
      [],
    ),
  });
}

As you can see, this code becomes very ugly very quickly. Even worse, the code becomes less performant. Why? Well, because memoization comes with a cost. And the more that memoization is used, the more costs users will have to pay. Yes, memoization is helpful. Yes, under the right circumstances, memoization can bring performance benefits. But it should be used as little as possible and only where it's truly needed.

Here in our example, we're performing 3 memoizations for a single function call. (One for defaultErrors, one for renderer, and one for useFormValidityObserver's internal call to useMemo.) That's not good. A better solution is to memoize the entire result once. But wait... Couldn't we just do that with createFormValidityObserver?

import { useMemo } from "react";
import { createFormValidityObserver } from "@form-observer/react";

function MyForm() {
  const { configure, ...rest } = useMemo(() => {
    return createFormValidityObserver("focusout", {
      revalidateOn: "input",
      renderer(errorContainer, errorMessage) {
        // Implementation
      },
      defaultErrors: {
        // ... Default error message configuration
      },
    });
  }, []);
}

In fact, this is more performant than useFormValidityObserver! If we memoize the result from useFormValidityObserver, then we perform 2 memoizations. (One for useFormValidityObserver's internal call to useMemo, and one for our own call to useMemo on the function's result). But if we simply memoize the result of createFormValidityObserver directly, then we only have to perform 1 memoization.

What does all of this mean? Well... It means that by default, even in the most ideal scenario, useFormValidityObserver will never be more performant than calling useMemo on createFormValidityObserver directly. And in most cases, useFormValidityObserver will be less performant. Moreover, many developers won't even be aware of the concerns that we've discussed so far; so they're much more likely to footgun themselves if they use the flashy useFormValidityObserver hook!

We wanted to find a way to protect developers from useMemo, but we couldn't. (And honestly, no developer who wants to be skilled with React can avoid learning about useMemo in this day and age -- not until React Forget stabilizes, at least.) This dilemma is just an unavoidable downside of how React decides to operate as a framework, and we sadly can't do anything to fix that. What we can do is guide developers on how to use createFormValidityObserver in React easily and performantly. And that's exactly what we've decided to do instead: Add clearer documentation as a better alternative.

Thankfully, things really aren't that complicated. People just need to wrap createFormValidityObserver inside useMemo (when using functional components). That's it.

Sidenote: Although we're removing the useFormValidityObserver hook, we aren't removing React as a peer dependency [yet]. The reason for this is that we're trying to figure out how to prepare for React 19's changes to ref callbacks (for the sake of autoObserve), and we're debating using the React package to help us with this. For example, we could use the React package to tell us the React version being used. Then, we could return one kind of autoObserve function if the React version is less than 19, and return a different kind of autoObserve function if the React version is greater than or equal to 19. But maybe that's a bad idea? Maybe we should just maintain different versions of our @form-observer/react package? Then again, it might not be a bad idea either since it seems like import { version } from "react" is permitted (at least as of today). We'll see...

…tyObserver`

Although this is a breaking change, it will more easily
allow us to support the `revalidateOn` feature that
we mentioned on GitHub while also maintaining the
parent-child relationship between the `FormObserver` and
the `FormValidityObserver`. (We could technically choose
to break that relationship, but the reusable `observe`
and `unobserve` methods seem to be proving helpful right
now.)

This change also seems more logical because -- practically
speaking -- developers are likely going to choose one
event anyway. They'll have to choose an event that actually
relates to user interaction. And the most common of those
events are `input`, `focusout`, and `change`. However,
the final `input` triggers before `focusout` happens, so
it doesn't make sense to specify both simultaneously.
Similarly, the `change` event will only be relevant for
a subset of the times that the `focusout` event is
triggered, so supplying those two simultaneously also
doesn't make sense. So developers will likely only
supply 1 event at a time.
This is not all of the ESLint packages. We're only updating
a few. The `@typescript-eslint` packages are of particular
interest since our previous version did not support
`[email protected]`.
This "Manual Mode" can be enabled by passing `null` to the `type`
argument of the constructor. The main reason that this is useful
is that it gives developers a way to only validate their forms
`onsubmit` if they so please.
This feature will be especially helpful for developers
who only want to validate their fields `oninput` _after_
their form has already been submitted.
We forgot about these limitations... And because we forgot about these
limitations, we ran off and tried to remove the `FormStorageObserver.d.ts`
and `FormValidityObserver.d.ts` files... But then we saw the problems
that came up when TypeScript attempted to generate these files automatically
from the JS files... We're not interested in enduring that headache again.

Note that even if we remove generic constructors from the
`FormStorageObserver` and the `FormValidityObserver`, TypeScript will still
get confused.

We've further documented why we're stuck in this predicament, and we hope
the TypeScript team will pull through for us at some point.
Includes a potential feature idea for the `FormValidityObserver`.
This feature would be easy to support and is likely not a super
high priority (unless users make it one).
We missed some of these documentation updates/improvements while
updating our project. Hopefully the docs are better now, though
surely there are other improvements that could be made.
Originally, this hook seemed like a good idea. But actually, it's
turned out to be something less performant than expected, and
something that's more prone to footgunning than expected. See the
`Design Decisions` document for additional details.

Notice that `react` is still a peer dependency of `@form-observer/react`.
In the future, we might use the React package to spy on the
React version so that we can figure out how to appropriately
generate the `autoObserve` function. If we decide against that,
we can remove the peer dependency. Again, see the `Design Decisions`
document for additional details.
@ITenthusiasm ITenthusiasm merged commit 62793c7 into next May 5, 2024
4 checks passed
@ITenthusiasm ITenthusiasm deleted the feat/revalidation branch May 5, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant